Conversation
dtolnay
left a comment
There was a problem hiding this comment.
Thanks, this mostly looks good except for the API of rust::MutSlice. The C++ API is going to need some thought because e.g. copying a &mut [T] is UB so a copy constructor probably can't exist.
It may also be worth considering rust::Slice<const T> for the const one and rust::Slice<T> for the non-const one.
|
I mean, you could just copy it by getting the data pointer and constructing another slice from it anyway. But yeah, technically that is UB so maybe the copy constructor shouldn't exist. Oh yeah, being able to reuse the Slice type for both mutable and immutable slices would be pretty nice. |
|
Hmm, the tests are failing because rust::repr::PtrLen has a Edit: also not seeing how to have the copy constructor only be available for |
Looks like a job for https://en.cppreference.com/w/cpp/language/const_cast. Alternatively maybe if PtrLen were to contain
Yeah something with enable_if. I'm not a C++ pro either, one of us will just need to poke on it until it works. -.- |
|
Well, this should work now. At least the tests pass, I can't check it with real code yet though because of #496. |
|
I can't get the enable_if on the slice constructors to work for some reason, and at this point I think I tried 7 different ways of doing it. It either works for both mutable and const slices or not at all. At this point I'd just leave it since C++ is already unsafe (even if that constructor was gone, you could still trivially copy mutable slices) unless you really want this check. Everything else seems to work now though, after testing it with the project I'm using cxx with. |
This seems to work, but I have no idea if it's the correct way to implement it.
Closes #451.